Skip to content

fix(798): trap Int/Nat arithmetic overflow + Tier-1 obligation (v0.0.181)#809

Merged
aallan merged 5 commits into
mainfrom
fix/798-int-overflow
Jun 27, 2026
Merged

fix(798): trap Int/Nat arithmetic overflow + Tier-1 obligation (v0.0.181)#809
aallan merged 5 commits into
mainfrom
fix/798-int-overflow

Conversation

@aallan

@aallan aallan commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Closes #798 — batch 4 of the #392 smt.py soundness audit.

The bug

vera verify modelled @Int / @Nat as Z3's unbounded integers, so + / - / * were treated as total — it proved postconditions the i64 / u64 runtime then violated under two's-complement wraparound:

public fn inc(@Int -> @Int)
  requires(true) ensures(@Int.result > @Int.0) effects(pure)
{ @Int.0 + 1 }

vera verify → proved for all inputs, yet vera run inc -- 9223372036854775807 (i64.MAX) wraps x + 1 to i64.MIN < x. Overflow was the one partial-arithmetic case treated as total by both tiers (@Nat underflow and signed-div MIN/-1 already trap).

The fix (a language-semantics decision — trap-on-overflow, approved)

@Int / @Nat + / - / * are now partial operations that trap on overflow at runtime, and the verifier auto-synthesises an int_overflow obligation that the result stays in i64 / u64 range:

  • Verifier (vera/verifier.py): a two-check mirroring array bounds (Auto-inject obligations for primitive operations (division, modulo, array index) to match README's static-safety claim #680) — provably in range → Tier 1; provably out of range (i64.MAX + 1) → loud E528; dynamic operands → runtime-guarded Tier 3. Lenient (tier3-by-default), so unguarded a + b is not a hard error.
  • Codegen (vera/wasm/operators.py): an explicit overflow-detection guard per op (WASM wraps; there is no native trap) — signed-i64 sign tests for Int add/sub, a division round-trip with a==0 + a==-1 && b==INT_MIN pre-checks for Int mul (dodging the native div_s trap), unsigned carry/round-trip for Nat add/mul. Traps via unreachable (the precise overflow kind is deferred — see below).
  • Classification at the operands' common (coerced) type@Int if either operand is @Int (since @Nat <: @Int), else @Nat. This is not one operand's self-type (a non-negative literal is @Nat, but 5 + @Int.0 is an i64 add) nor the possibly-narrowed result (@Int.0 + 1 stored into a @Nat slot is still an i64 add). Verifier and codegen share this rule in lockstep.

@Nat subtraction is excluded — it stays the existing underflow obligation (E502).

Exit checklist (§0)

  • pytest tests/ — green (5124 passed, 16 skipped, 16 deselected)
  • mypy vera/ — clean
  • check_conformance.py — 93/93 (new ch04_int_overflow.vera, all sites Tier-1)
  • check_examples.py — 35/35 (check + verify)
  • check_version_sync.py — 0.0.181
  • CHANGELOG [0.0.181] + link refs; doc-counts / site-assets / spec-examples green
  • New behaviour covered by tests that fail on the pre-fix verifiertests/test_int_overflow.py (RED-confirmed)

Verification (test-first + adversarial)

  • The codegen WASM sequences were designed via a workflow (adversarial scrutiny of the error-prone Int-MUL round-trip) then implemented test-first.
  • MUL random differential: 20,000 random i64 pairs + 20,000 u64 pairs — codegen-traps(a*b) ⟺ Python's exact product out of range, 0 mismatches.
  • Verifier↔codegen differential (tests/test_int_overflow_differential.py): the set of int_overflow-obligated sites == the set of codegen-guarded sites, swept across every example + verify/run conformance program.
  • The operand classification went through three iterations (operand self-type → result type → operands' LUB), each exposed by a corpus probe comparing the classification against the true arithmetic type. The lockstep differential was green for all three — agreement ≠ correctness — so the regression tests now pin the literal-left and narrowed-result shapes against ground truth.

Notable change

To classify in lockstep, the checker's resolved-type side-table is now threaded into codegen (vera/wasm/context.py, vera/codegen/{api,core,functions,closures}.py, vera/cli.py), so vera compile / vera run consult the same resolved type the verifier does.

Deferred (sound, tracked)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added verified @Int/@Nat arithmetic overflow obligations for +, -, and *, with dynamic overflow enforced by runtime traps.
  • Bug Fixes
    • Aligned overflow handling and diagnostics (including E528) across verification and code generation, including correct @Nat subtraction behaviour.
  • Documentation
    • Updated spec, error-code docs, and conformance/testing documentation to reflect the expanded suite (93 programs), plus the 0.0.181 changelog entry.
  • Tests
    • Added and updated verifier, codegen, differential lockstep, and conformance/regression tests for overflow trapping and obligation accounting.
  • Chores
    • Bumped the package version to 0.0.181.

…181)

vera verify modelled Int/Nat as Z3 unbounded integers, so +, -, and *
were total — it proved postconditions the i64/u64 runtime violated under
two's-complement wraparound (ensures(result > x) for { x + 1 } verified at
Tier 1, yet inc(i64.MAX) wraps to i64.MIN). Overflow was the one
partial-arithmetic case treated as total by both tiers; Nat underflow and
signed-division MIN/-1 already trap.

Int/Nat +, -, and * are now partial operations that trap on overflow at
runtime, with an auto-synthesised int_overflow obligation that the result
stays in i64/u64 range — a two-check mirroring array bounds (#680):
provably in range -> Tier 1; provably out of range -> compile error E528;
dynamic operands -> runtime-guarded Tier 3 (lenient, so unguarded a + b is
not a hard error). Overflow is classified at the operands' common coerced
type (Int if either operand is Int, since Nat <: Int) — not one operand's
self-type, nor the possibly-narrowed result. Nat subtraction stays the
existing underflow obligation (E502).

Codegen emits explicit WASM overflow-detection guards (WASM wraps; there is
no native trap), trapping via unreachable; the precise overflow trap kind is
deferred to #808. The checker's resolved-type side-table is now threaded
into codegen so vera compile / vera run classify in lockstep with the
verifier.

Closes #798.

Co-Authored-By: Claude <noreply@anthropic.invalid>
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 728c5dd0-8516-476c-9011-897a5f1392c4

📥 Commits

Reviewing files that changed from the base of the PR and between 20bf85c and c7b5ada.

📒 Files selected for processing (1)
  • spec/11-compilation.md

📝 Walkthrough

Walkthrough

This PR adds integer-overflow obligations and runtime guards for @Int/@Nat +, -, and *, threads resolved expression types from type checking into codegen, and updates verifier/runtime tests, docs, and version metadata for the new E528 behaviour and 93-program conformance suite.

Changes

Integer overflow soundness

Layer / File(s) Summary
Overflow contract surface
vera/obligations/core.py, vera/errors.py, spec/06-contracts.md, spec/11-compilation.md, vera/verifier.py, CHANGELOG.md, HISTORY.md, ROADMAP.md
int_overflow, E528, and overflow semantics for @Int/@Nat arithmetic are added to the public contract, spec text, verifier logic, and release/status notes.
Type-flow plumbing
vera/cli.py, vera/codegen/api.py, vera/codegen/core.py, vera/codegen/closures.py, vera/codegen/functions.py, vera/wasm/context.py
expr_semantic_types now flows from type checking through compile() and CodeGenerator into WasmContext for overflow classification.
WASM overflow guards
vera/wasm/operators.py, tests/test_codegen.py
Binary +, -, and * now emit signed and unsigned overflow guards, with Nat subtraction left on the existing underflow path.
Verifier tests
scripts/check_spec_examples.py, tests/conformance/manifest.json, tests/test_int_overflow.py, tests/test_verifier.py, TESTING.md
New verifier regression tests and updated tier counts cover int_overflow obligations, and the conformance manifest and example allowlist gain the new overflow program.
Runtime tests
tests/test_int_overflow_codegen.py
Boundary-case runtime tests cover trapping and non-trapping behaviour for @Int and @Nat arithmetic.
Differential corpus test
tests/test_int_overflow_differential.py
A corpus-wide lockstep test compares verifier and codegen overflow classification for arithmetic binary sites.
Docs and release metadata
AGENTS.md, CLAUDE.md, FAQ.md, README.md, SKILL.md, TESTING.md, pyproject.toml, vera/__init__.py
Project docs, testing docs, and version metadata update the version, conformance counts, test totals, and overflow references.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90+ minutes

Possibly related issues

Possibly related PRs

  • aallan/vera#554: Introduces the earlier @Nat subtraction underflow guard path that this overflow work distinguishes from.

Suggested labels

compiler, tests, spec, ci, docs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.35% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarises the main change: trapping Int/Nat arithmetic overflow with a new Tier-1 obligation.
Linked Issues check ✅ Passed The PR matches #798 by adding verifier obligations, runtime overflow traps, type-threaded codegen, and regression coverage for Int/Nat overflow.
Out of Scope Changes check ✅ Passed The remaining documentation, changelog, and version updates support the overflow fix and release bump, with no obvious unrelated changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/798-int-overflow

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.55172% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.03%. Comparing base (06063cf) to head (c7b5ada).

Files with missing lines Patch % Lines
vera/wasm/operators.py 94.79% 5 Missing ⚠️
vera/verifier.py 98.46% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #809      +/-   ##
==========================================
+ Coverage   91.95%   92.03%   +0.07%     
==========================================
  Files          89       89              
  Lines       26684    26853     +169     
  Branches      321      321              
==========================================
+ Hits        24537    24713     +176     
+ Misses       2139     2132       -7     
  Partials        8        8              
Flag Coverage Δ
javascript 65.33% <ø> (ø)
python 95.08% <96.55%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@spec/06-contracts.md`:
- Around line 247-248: The E528 obligations table entry is too broad because it
implies `@Nat - `@Nat`` is included, while the verifier behavior keeps `@Nat`
subtraction under E502 only. Update the summary row in the contracts spec to
narrow the arithmetic covered by E528 or explicitly add the `@Nat - `@Nat``
exception, and make sure the wording stays consistent with the later paragraph
in the same section.

In `@spec/11-compilation.md`:
- Around line 56-57: The overflow paragraph needs to explicitly describe the
E528 verifier branch instead of folding it into the Tier-3 fallback. Update this
section to spell out the three cases in the verifier/codegen flow: proven
overflow raises E528 before codegen, solver-uncertain cases fall back to Tier 3
with guarded trapping ops, and proven-in-range cases emit the plain WASM op.
Make the classification key off the operands’ common coerced arithmetic type,
and reference the existing verification terminology so it matches the behavior
described elsewhere in the specification.

In `@TESTING.md`:
- Around line 9-13: The conformance program count is inconsistent across
TESTING.md, with the overview updated to 93 while another literal still says 92.
Update every remaining 92 in the document so all references match the new
conformance count, especially the later Conformance Suite prose that should
align with the table entry.

In `@tests/test_int_overflow_codegen.py`:
- Around line 43-67: The _compile_with_types helper only typechecks the direct
source and bypasses ModuleResolver, so it does not cover overflow classification
in imported bodies like cmd_run/cmd_compile do. Update this helper to mirror the
production import-resolution flow by resolving modules before codegen, then pass
the resulting artifacts through typecheck_with_artifacts and codegen_compile
using expr_semantic_types from the resolved compilation context. Keep the fix
centered on _compile_with_types and the imported-path setup so the suite
exercises cross-module overflow cases.

In `@tests/test_int_overflow_differential.py`:
- Around line 268-294: The narrowed `@Int-to-`@Nat overflow case is only covered
by verifier-only assertions and is missing from the parametrized
codegen-vs-verifier lockstep. Add this shape to _CORPUS so it is exercised
through _assert_in_lockstep, using the existing ContractVerifier and overflow
classification helpers to confirm the arithmetic width remains Int while the
result type is Nat.

In `@tests/test_int_overflow.py`:
- Around line 45-83: The overflow tests are too permissive because they only
check that at least one `int_overflow` obligation exists, so duplicate emissions
from the walker could still pass. Tighten the assertions in
`test_int_add_unbounded_emits_undischarged_overflow_obligation`,
`test_nat_add_unbounded_emits_undischarged_overflow_obligation`, and
`test_overflow_obligation_discharged_when_operands_bounded` by asserting the
exact overflow-obligation count or the exact `(kind, status)` sequence from
`result.obligations`, using the existing `_verify` helper and `ovf` filtering.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bcbcd9b3-6219-40ec-9469-bd54178cbe52

📥 Commits

Reviewing files that changed from the base of the PR and between 06063cf and ffb4f93.

⛔ Files ignored due to path filters (7)
  • docs/SKILL.md is excluded by !docs/**
  • docs/index.html is excluded by !docs/**
  • docs/index.md is excluded by !docs/**
  • docs/llms-full.txt is excluded by !docs/**
  • docs/llms.txt is excluded by !docs/**
  • tests/conformance/ch04_int_overflow.vera is excluded by !**/*.vera
  • uv.lock is excluded by !**/*.lock, !uv.lock
📒 Files selected for processing (30)
  • AGENTS.md
  • CHANGELOG.md
  • CLAUDE.md
  • FAQ.md
  • HISTORY.md
  • README.md
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • pyproject.toml
  • scripts/check_spec_examples.py
  • spec/06-contracts.md
  • spec/11-compilation.md
  • tests/conformance/manifest.json
  • tests/test_codegen.py
  • tests/test_int_overflow.py
  • tests/test_int_overflow_codegen.py
  • tests/test_int_overflow_differential.py
  • tests/test_verifier.py
  • vera/__init__.py
  • vera/cli.py
  • vera/codegen/api.py
  • vera/codegen/closures.py
  • vera/codegen/core.py
  • vera/codegen/functions.py
  • vera/errors.py
  • vera/obligations/core.py
  • vera/verifier.py
  • vera/wasm/context.py
  • vera/wasm/operators.py

Comment thread spec/06-contracts.md Outdated
Comment thread spec/11-compilation.md Outdated
Comment thread TESTING.md Outdated
Comment thread tests/test_int_overflow_codegen.py
Comment thread tests/test_int_overflow_differential.py
Comment thread tests/test_int_overflow.py
… doc accuracy

Round 2: adversarial-review + CodeRabbit findings on PR #809 (all tests/docs;
no core-logic change).

Tests:
- Add the missing E528 'violated' regression (Int + Nat) — the loud-error arm of
  the two-check was untested; mutation-validated (disabling the branch flips it
  RED). Tighten the three existing obligation tests to assert the exact count.
- Pin the Nat-subtraction exclusion at the real verifier gate (nat_sub, not
  int_overflow), not just the differential's re-derived helper.
- Add the narrowed Int-into-Nat shape to the codegen/verifier lockstep corpus.
- Cross-module coverage: thread ModuleResolver through the codegen helper
  (mirroring cmd_run) + a test that an imported function's overflow traps;
  mutation-validated against the guard hook.

Docs:
- spec/11 section 11.2.1: codegen is type-driven, not tier-driven — it emits the
  guard at every classified site regardless of the Z3 tier (fixed for both the
  overflow paragraph and the parallel Nat-subtraction paragraph); spell out the
  verifier three-way split (Tier 1 / E528 / Tier 3); drop the bare "(Option A)".
- spec/06 section 6.4.3: narrow the E528 table row so Nat subtraction (E502) is
  not implied; fix the wrong "i64.MAX + 1" example (bare literals are Nat, in
  range) to "u64.MAX + 1". Same example fix in CHANGELOG.
- TESTING.md: 92 -> 93 conformance count (3 stale mentions); refresh test counts
  (5145 -> 5151) across TESTING / README / ROADMAP.

Refs #798.

Co-Authored-By: Claude <noreply@anthropic.invalid>
@aallan

aallan commented Jun 27, 2026

Copy link
Copy Markdown
Owner Author

Review summary — adversarial multi-agent + CodeRabbit

The change was reviewed two independent ways:

  1. Adversarial multi-dimension review — 7 lenses (verifier↔codegen lockstep, codegen correctness, verifier two-check, tests, comments/docs, silent failures, general code), each finding then re-verified with an actual repro. Zero confirmed defects in the core verifier/codegen logic — the lockstep, codegen-correctness, verifier-two-check, and silent-failure lenses all came back clean. 5 confirmed findings, all in tests/docs.
  2. CodeRabbit — 6 actionable comments, all Minor/Trivial, all in tests/docs.

All findings (deduped to 9 distinct items) are addressed in 0c5c56d. No core-logic changes this round.

Test coverage

  • Added the missing E528 "provably overflows" regression — the loud-error arm of the two-check had no test (mutation-validated: disabling the verifier branch leaves the rest of the suite green, but flips these RED). @Int + @Nat variants.
  • Pinned the @Nat-subtraction exclusion at the real verifier gate (nat_sub, not int_overflow) — previously only re-derived inside the differential helper, which couldn't catch an exclusion desync.
  • Added the narrowed @Int@Nat shape to the codegen↔verifier lockstep _CORPUS (was verifier-only).
  • Added cross-module coverage: an imported function's overflow now traps in a test (mutation-validated against the guard hook); the codegen helper threads ModuleResolver like cmd_run/cmd_compile.
  • Tightened the three existing obligation tests to assert the exact obligation count.

Doc accuracy

  • spec §11.2.1: codegen is type-driven, not tier-driven — the guard is emitted at every classified site regardless of the Z3 tier (a Tier-1 discharge means it provably never fires, but it is still emitted). Corrected for both the overflow paragraph and the parallel @Nat-subtraction paragraph; spelled out the verifier three-way split (Tier 1 / E528 / Tier 3); dropped a dangling "(Option A)".
  • spec §6.4.3 + CHANGELOG: narrowed the E528 table row so @Nat subtraction stays E502, and fixed the i64.MAX + 1 example — bare literals are @Nat, and that sum fits u64, so it does not overflow — to u64.MAX + 1.
  • TESTING.md: conformance 9293; test counts refreshed across TESTING / README / ROADMAP.

@aallan

aallan commented Jun 27, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@spec/11-compilation.md`:
- Around line 56-57: The subtraction summary in this section still overstates
the rule by implying non-@Nat subtraction sites are not obligation-checked,
which conflicts with the new `@Int/`@Nat overflow description. Update the
neighboring prose in the compilation spec so the follow-on sentence applies only
to `@Nat` subtraction underflow, while keeping the `@Int/`@Nat addition,
subtraction, and multiplication overflow handling described by the existing
arithmetic-obligation paragraph and its Tier 1/Tier 3 behavior.

In `@tests/test_int_overflow_codegen.py`:
- Around line 297-345: The overflow regression suite is missing coverage for the
special `@Nat` subtraction path, so add a small subtraction test block alongside
TestNatAddOverflow798 and TestNatMulOverflow798. Use the existing _assert_traps
and _assert_no_trap helpers with the `@Nat` subtraction opcode path (the
nat_sub/E502 behavior) to verify 0 - 1 traps and 5 - 3 stays in range. Keep the
new tests focused on the `@Nat` subtraction contract so codegen cannot silently
switch it to the new overflow-guarded arithmetic behavior.

In `@tests/test_int_overflow_differential.py`:
- Around line 323-343: The corpus sweep is skipping non-standalone files and
only checking binary sites from the entry AST, so imported-module arithmetic is
never compared. Update the test flow around typecheck_with_artifacts,
ContractVerifier, and _binary_sites to resolve imports via ModuleResolver
instead of pytest.skip, then collect and include binary sites from imported
modules in the mismatch loop so the lockstep check covers cross-module cases
too.

In `@tests/test_int_overflow.py`:
- Around line 108-119: The Nat ceiling overflow test currently only checks the
violated `int_overflow` obligation in
`test_nat_overflow_obligation_violated_at_u64_ceiling`, so extend it to also
assert the user-facing `E528` diagnostic for this `@Nat` path. Reuse the same
diagnostic-checking pattern already used in the signed-ceiling overflow test,
and verify the public verifier output alongside the existing
`result.obligations` assertions so the Nat branch covers both internal and
external surfaces.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a21b47bf-6034-4006-9f71-aeb4c6290784

📥 Commits

Reviewing files that changed from the base of the PR and between 06063cf and 0c5c56d.

⛔ Files ignored due to path filters (7)
  • docs/SKILL.md is excluded by !docs/**
  • docs/index.html is excluded by !docs/**
  • docs/index.md is excluded by !docs/**
  • docs/llms-full.txt is excluded by !docs/**
  • docs/llms.txt is excluded by !docs/**
  • tests/conformance/ch04_int_overflow.vera is excluded by !**/*.vera
  • uv.lock is excluded by !**/*.lock, !uv.lock
📒 Files selected for processing (30)
  • AGENTS.md
  • CHANGELOG.md
  • CLAUDE.md
  • FAQ.md
  • HISTORY.md
  • README.md
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • pyproject.toml
  • scripts/check_spec_examples.py
  • spec/06-contracts.md
  • spec/11-compilation.md
  • tests/conformance/manifest.json
  • tests/test_codegen.py
  • tests/test_int_overflow.py
  • tests/test_int_overflow_codegen.py
  • tests/test_int_overflow_differential.py
  • tests/test_verifier.py
  • vera/__init__.py
  • vera/cli.py
  • vera/codegen/api.py
  • vera/codegen/closures.py
  • vera/codegen/core.py
  • vera/codegen/functions.py
  • vera/errors.py
  • vera/obligations/core.py
  • vera/verifier.py
  • vera/wasm/context.py
  • vera/wasm/operators.py

Comment thread spec/11-compilation.md Outdated
Comment thread tests/test_int_overflow_codegen.py
Comment thread tests/test_int_overflow_differential.py
Comment thread tests/test_int_overflow.py
…oss-module test coverage

Round 3: 4 CodeRabbit findings on PR #809 (all tests/docs; no core-logic change).

- spec/11 section 11.2.1: scope the "Int - Int not obligation-checked" sentence
  to the *underflow* obligation and cross-reference the overflow obligation above
  — the flat wording contradicted the new overflow paragraph.
- tests/test_int_overflow.py: assert the user-facing E528 diagnostic on the Nat
  ceiling test too (it only checked the obligation), mirroring the Int test.
- tests/test_int_overflow_codegen.py: add TestNatSubUnderflow798 pinning that Nat
  subtraction stays the #520 nat_sub underflow guard, not #798 overflow
  arithmetic. CR's suggested "0 - 1" example is exempt (pure literals don't
  trap); the tests use slot provenance (3 - 5 traps, 5 - 3 = 2).
- tests/test_int_overflow_differential.py: cover cross-module arithmetic by
  sweeping imported *_lib.vera modules STANDALONE. CR's suggested fix (resolve
  imports in the consumer + include imported sites) is a false positive: the
  verifier obligates per-module, so verifier-of-consumer (None) vs
  codegen-of-imported-body (Int) is a category mismatch, not a desync. Swept
  standalone, the lib's sites are in lockstep.

Doc counts refreshed (5151 -> 5156).

Refs #798.

Co-Authored-By: Claude <noreply@anthropic.invalid>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
spec/11-compilation.md (1)

46-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Synchronise §11.3.3 with the always-guarded @Nat subtraction rule.

This subsection now correctly says the @Nat - @Nat`` guard is emitted regardless of verifier tier, but Line 109 still says a discharged site compiles to bare i64.sub. That leaves the spec internally inconsistent on the same codegen path. Please update the later operator note so Tier 1 means “guard proven unreachable”, not “unguarded op emitted”. Based on learnings, "`Nat` subtraction guards and `Int`/`Nat` overflow guards are type-driven rather than tier-driven". As per path instructions, "Review Markdown files for factual accuracy against the codebase".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@spec/11-compilation.md` around lines 46 - 58, The §11.3.3 operator note is
inconsistent with the always-guarded `@Nat - `@Nat`` codegen path described in the
same section. Update the subtraction/overflow wording in this spec text so Tier
1 means the guard is proven unreachable, not that `i64.sub` is emitted without a
guard, and keep the explanation aligned with the `@Nat` subtraction and
arithmetic overflow rules referenced in the surrounding prose.

Sources: Path instructions, Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@spec/11-compilation.md`:
- Around line 46-58: The §11.3.3 operator note is inconsistent with the
always-guarded `@Nat - `@Nat`` codegen path described in the same section. Update
the subtraction/overflow wording in this spec text so Tier 1 means the guard is
proven unreachable, not that `i64.sub` is emitted without a guard, and keep the
explanation aligned with the `@Nat` subtraction and arithmetic overflow rules
referenced in the surrounding prose.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: aad6426d-bbe6-4488-943b-74ba2799c14f

📥 Commits

Reviewing files that changed from the base of the PR and between 0c5c56d and 6412a2c.

📒 Files selected for processing (7)
  • README.md
  • ROADMAP.md
  • TESTING.md
  • spec/11-compilation.md
  • tests/test_int_overflow.py
  • tests/test_int_overflow_codegen.py
  • tests/test_int_overflow_differential.py

…iven codegen

Addresses a CodeRabbit outside-diff finding on PR #809: section 11.3.3 still said
a Z3-discharged Nat subtraction compiles to a bare i64.sub, contradicting the
section 11.2.1 rule (corrected earlier this PR) that the underflow guard is
emitted regardless of the verifier's tier. Reworded: the bare i64.sub applies
only at guard-exempt sites (pure-literal subtractions); where the guard applies
it is emitted type-driven (a Tier-1 discharge means it provably never fires, not
that it is omitted).

Refs #798.

Co-Authored-By: Claude <noreply@anthropic.invalid>
@aallan

aallan commented Jun 27, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai The outside-diff finding on spec/11-compilation.md §11.3.3 is fixed in 20bf85c. Reworded so the bare i64.sub applies only at guard-exempt sites (pure-literal subtractions); where the underflow guard applies it is emitted regardless of the verifier's tier — a Tier-1 discharge of lhs >= rhs means the guard provably never fires, not that the bare op is emitted instead. This brings §11.3.3 in line with the §11.2.1 rule corrected earlier in this PR (codegen is type-driven, not tier-driven).

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@aallan Thanks — that resolves my concern.

The rewording for spec/11-compilation.md §11.3.3 is consistent with the corrected §11.2.1 rule: guard emission is driven by the arithmetic/type classification, not by verifier tier, and a Tier 1 discharge only proves the guard is unreachable at runtime rather than changing codegen to the bare op.


✏️ Learnings added
Learnt from: aallan
URL: https://github.com/aallan/vera/pull/809

Timestamp: 2026-06-27T07:42:52.266Z
Learning: In this Vera compiler codebase, the compilation semantics described in `spec/11-compilation.md` are type-driven rather than verifier-tier-driven: for guarded arithmetic such as `Nat` subtraction underflow and `Int`/`Nat` overflow checks, the guard is still emitted at classified sites even when the verifier discharges the obligation at Tier 1; Tier 1 means the guard is provably never taken, not that codegen switches to an unguarded bare operation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
spec/11-compilation.md (1)

46-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use i64 consistently here
@Int and @Nat both compile to i64 in §11.2.1, so the i64/u64 wording is off. The lead-in should also be narrowed to the underflow-specific @Nat - @Nat`` case now that +, `-`, and `*` all have overflow obligations.

Suggested tweak
- The one operation that can violate the invariant despite well-typed operands is unsigned subtraction (`@Nat - `@Nat``).
+ The underflow-specific case is unsigned subtraction (`@Nat - `@Nat``).

- ... (`@Int` (i64) if either operand is `@Int`, else `@Nat` (u64); `@Nat` subtraction is excluded, being the underflow obligation above).
+ ... (`@Int` if either operand is `@Int`, else `@Nat`; `@Nat` subtraction is excluded, being the underflow obligation above).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@spec/11-compilation.md` around lines 46 - 56, The §11.2.1 arithmetic wording
is inconsistent: `@Int` and `@Nat` both lower to `i64`, so remove the `i64/u64`
split and describe the common representation as `i64` throughout. Also narrow
the lead-in to the underflow-specific `@Nat - `@Nat`` case, since the overflow
discussion for `+`, `-`, and `*` should be described separately. Update the
relevant text in the compilation spec section that mentions the subtraction
obligation and the common arithmetic type so it matches the `@Int`/`@Nat`
lowering used by the compiler.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@spec/11-compilation.md`:
- Around line 46-56: The §11.2.1 arithmetic wording is inconsistent: `@Int` and
`@Nat` both lower to `i64`, so remove the `i64/u64` split and describe the
common representation as `i64` throughout. Also narrow the lead-in to the
underflow-specific `@Nat - `@Nat`` case, since the overflow discussion for `+`,
`-`, and `*` should be described separately. Update the relevant text in the
compilation spec section that mentions the subtraction obligation and the common
arithmetic type so it matches the `@Int`/`@Nat` lowering used by the compiler.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4e7e25f7-d699-44d3-8402-20d5f8a2af79

📥 Commits

Reviewing files that changed from the base of the PR and between 6412a2c and 20bf85c.

📒 Files selected for processing (1)
  • spec/11-compilation.md

…gned/unsigned 64-bit)

Addresses a second CodeRabbit outside-diff finding on PR #809: section 11.2.1
said Int/Nat "share the i64/u64 representation", contradicting the section's own
opening ("share the same WASM representation (i64)"). Both lower to i64; the u64
is the unsigned interpretation/range, not a separate representation.

- Representation: both are i64 (one WASM type).
- Overflow range: signed 64-bit for Int, unsigned 64-bit for Nat — the
  load-bearing distinction (the verifier uses I64_MIN/I64_MAX vs 0/U64_MAX),
  now stated as a range, separate from representation.
- Scoped the lead-in to the *non-negativity* invariant so the underflow
  (Nat - Nat) discussion reads as distinct from the overflow discussion below.

Refs #798.

Co-Authored-By: Claude <noreply@anthropic.invalid>
@aallan

aallan commented Jun 27, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai The §11.2.1 representation/range finding is fixed in c7b5ada. You're right that @Int and @Nat share the i64 representation — I dropped the "i64/u64 representation" phrasing (it contradicted the section's own opening, "share the same WASM representation (i64)"). I kept the i64-vs-u64 distinction only where it's load-bearing: the overflow range genuinely differs — signed 64-bit for @Int, unsigned 64-bit for @Nat (the verifier checks I64_MIN/I64_MAX vs 0/U64_MAX) — now stated explicitly as a range, separate from the representation. Also scoped the lead-in to the non-negativity invariant so the @Nat - @Nat underflow reads as distinct from the overflow discussion below.

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@aallan Thanks — that resolves my concern.

The revised wording makes the distinction precise: @Int and @Nat share the same WASM representation (i64), while the verifier’s overflow reasoning differs by range (I64_MIN..I64_MAX for @Int vs 0..U64_MAX for @Nat). Scoping @Nat - @Nat`` under the non-negativity invariant also makes the underflow-vs-overflow split much clearer.


✏️ Learnings added
Learnt from: aallan
URL: https://github.com/aallan/vera/pull/809

Timestamp: 2026-06-27T08:09:37.845Z
Learning: In this Vera compiler codebase, `Int` and `Nat` share the same WebAssembly representation (`i64`), but overflow checking uses different semantic ranges: `Int` is checked against signed 64-bit bounds (`I64_MIN..I64_MAX`) and `Nat` against unsigned bounds (`0..U64_MAX`). In `spec/11-compilation.md` §11.2.1, `Nat - Nat` should be framed under the non-negativity/underflow invariant, distinct from the overflow discussion.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@aallan aallan merged commit 2690a08 into main Jun 27, 2026
28 checks passed
@aallan aallan deleted the fix/798-int-overflow branch June 27, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

verifier/smt.py: integer arithmetic overflow unmodeled — Tier 1 proves contracts false under i64/u64 wraparound

1 participant